fix: content_overlap uses the real CSS box for out-of-flow text - #3923
Open
miga-heygen wants to merge 3 commits into
Open
miga-heygen wants to merge 3 commits into
miga-heygen wants to merge 3 commits into
Conversation
content_overlap measured text overlap via Range.getClientRects(), which reflects a font's own ascent/descent metrics, not the CSS box: a large font-size with a tight line-height keeps the same metrics-rect height regardless of line-height while the real box scales normally. An absolutely/fixed-positioned block has no layout-engine-reserved space (unlike an in-flow flex/grid pair, already exempted), so a genuine CSS-box gap between two such blocks could still measure as overlapping via font metrics -- a false collision report immediately followed by nothing actually unreadable on screen. Adds visibleBoxClientRects, using getBoundingClientRect() instead of Range rects (sharing the existing ancestor-overflow-clip logic via a new clipRectsToOverflowAncestors helper), and switches to it for out-of-flow elements only -- in-flow text keeps the existing font-metrics measurement. Deliberately narrower than "in-flow is always safe": the flex/grid exemption only covers a pair sharing the same flex/grid ancestor, not every in-flow pair, so an unrelated pair of ordinary in-flow blocks with the same font-size/line-height mismatch remains a residual gap, left for a follow-up rather than expanding this fix's scope. Co-Authored-By: Miguel Angel <miguel.sierra@heygen.com>
… box Replace the per-position geometry switch (bare border-box for out-of-flow blocks, raw glyph rects for in-flow) with one rule for every block: glyph rects, with half-leading spill past the element's own border-box trimmed back to the box. A side is trimmed only when the outermost line spills by at most half its own rect height, which bounds (content-area - line-height) / 2 exactly and is scale-invariant; a further wrapped line always spills more and is kept as real ink. Horizontal spill is never trimmed, and vertical writing modes are left untrimmed rather than trimming real overflow. The bare box over-reported when wider or taller than its text and compared unlike geometries within a mixed pair; an unbounded clamp dropped genuine overflow from unpainted boxes, which no overflow constraint owns. Share the per-rect intersection between the ancestor overflow clip and the box trim, and cover box-larger-than-text, text-overflowing-box, threshold boundaries, multi-line bleed, mixed and in-flow pairs, and vertical writing mode with fixtures whose mutants each fail. Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
Contributor
Author
|
Addressed in 6ddf660. |
Bound the border-box trim to a fifth of the spilling line's rect height instead of a half. Half-leading is (content-area - line-height) / 2, and at up to a fifth of the content area the box still encloses the cap band, so only ascender and descender tips lie outside it; a neighbour set against the box edge shares about 1% of its ink with them. Tighter than that, the box sits inside the cap band and the spill is dense glyph ink: in Chromium a neighbour at the box edge shares 22-42% of its ink from line-height 0.4 down to 2px, which the half bound trimmed away and no longer reported. Drop the redundant glyph union in favour of the outermost line edges, state negative leading explicitly in the rule comment, and pin the new bound from both sides, deep negative leading in 50px and 25px boxes, and bleed trimmed above the box with the neighbour above. Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
Contributor
Author
|
Addressed in 18d90db. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
content_overlap's text measurement (textClientRects, viadocument.createRange().selectNodeContents(el); range.getClientRects()) reflects a font's own vertical metrics (ascent/descent), not the CSS box (getBoundingClientRect()). A large font-size with a tight line-height keeps the range-rect height regardless of line-height, while the real box shrinks around it. In-flow text blocks sharing a flex/grid ancestor already can't collide (an existing exemption), but nothing else has that guarantee, so a genuine CSS-box gap between two blocks — typically absolutely positioned data-card labels — could still measure as overlapping via font metrics: a false collision report with nothing actually unreadable on screen.Fix
content_overlapnow measures every solid text block the same way (overlapTextRects, the single place the rule is decided): the glyph rects, with font-metric bleed past the element's own border-box trimmed back to the box, then clipped by overflow ancestors as before.Neither raw geometry is right on its own. Range rects over-report through half-leading; the bare box over-reports the other way (a box wider or taller than the text it holds would register collisions its glyphs never make, and swapping to the box for some blocks but not others compares unlike geometries within a pair). So the box is only used to trim spill that is sparse ink. With negative leading (line-height below the font's content area) each line spills
(content-area − line-height) / 2past each box edge. A side is trimmed only while that spill is at most a fifth of the line's own rect height, i.e. line-height at least 60% of the content area (about 0.7em): the box still encloses the x-height and cap band, only ascender/descender tips lie outside, and a neighbour set against the box edge shares about 1% of its ink with them. Any tighter and the box sits inside the cap band — the spill is dense glyph ink that a neighbour at the box edge collides with for real (over 20% shared ink from roughly 0.4em down) — so it is kept and still collides, as on main. A further wrapped line spills(line-height + content-area) / 2, always more, and is kept too. The bound is measured from the rect itself, so it holds under any transform scale (an entrance punch-in sampled mid-animation gives the same verdict as the resting frame). Horizontal spill is by nature never bleed and is never trimmed. This matters because a bare unpainted box is not an overflow constraint, so text spilling out of such a box into a neighbour is only ever reported bycontent_overlap.The per-rect intersection previously inlined in the ancestor overflow-clip loop is now a shared
clampRectsTohelper used by both that loop and the border-box trim.Known limitations, documented at the rule and left for follow-ups:
<strong>/<em>) is its own solid block, and an inline box already follows the font content area, so the trim does not help there; trimming against the nearest block-level ancestor's box would.isManagedFlowOverlap(the same-flex/grid waiver) still waives flex/grid children whose glyphs genuinely overlap (e.g. a negativemargin-topin a flex column, 29% shared ink) — pre-existing, unchanged here.clipRectsToOverflowAncestorsstarts at the parent, so an element's ownoverflow: hidden(line-clamp/truncation patterns) is not applied to its glyph rects — pre-existing.Test plan
New fixtures in
layout-audit.browser.test.ts(the overlap style mock previously reported noposition; it now defaults tostaticlike a real browser):writing-mode: vertical-rlcolumn whose text runs 200px past a 300px box → left untrimmed and still flagged (vertical modes swap the axes, so the clamp steps aside rather than trim real overflow).Mutation checks run against these fixtures: raw glyph rects (main) fails the bleed fixtures; bare box for out-of-flow blocks fails the box-larger-than-text and overflow fixtures; an unbounded clamp fails the overflow fixtures; a half-height bound fails 3 negative-leading fixtures; never trimming above fails 1; never trimming below fails 2.
Real-Chromium ink oracle (Liberation Sans 100px, stacked absolute blocks with the lower box starting at the upper box's bottom edge; shared ink as % of the smaller block's ink): line-height 0.7 → 1.4% shared, main flags, this branch clean (the false positive being fixed; same with a 10px box gap, under a scaled ancestor, on the last line of a multi-line block, and with the neighbour above). Line-height 0.8 headline with a caption at its box bottom → 9.1%, clean. Line-height 0.6 → 15.4%, 0.5 → 11.6%, 0.4 → 22.4%, 0.25 → 30.3%, 0.1 → 27.9%, 2px → 41.7%, 0.5 with a caption at the box edge → 24.7%: all flagged, matching main. Wide-box/tall-box short text → 0%, clean; genuine 30px box overlap, vertical-rl overflow, caption over descenders → flagged.
layout-audit.browser.test.tscannot be collected by the defaulthappy-domenvironment in this sandbox (No such built-in module: node:, pre-existing for the whole file); the full file was run instead under--environment nodewith globals populated from ahappy-domWindowviapopulateGlobal: 137 passed.tsc --noEmitclean forpackages/cli,oxlint/oxfmt --checkclean on both touched files.